Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Last commit refactors JavaScript execution by moving execute_element_script functionality from the Tab class to the WebElement class for better encapsulation.

I think this is too much. We can keep something with a minimal useful API, and for advanced users, they can execute the scripts directly.

For instance, something like this would be better:

 @overload
 async def execute_script(self, script: str, await_promise: bool = True, return_by_value: bool = True) -> EvaluateResponse: ...
 @overload
 async def execute_script(self, script: str, element: WebElement, await_promise: bool = True, return_by_value: bool = True) -> CallFunctionOnResponse: ...

I think this is too much. We can keep something with a minimal useful API, and for advanced users, they can execute the scripts directly.

For instance, something like this would be better:

 @overload
 async def execute_script(self, script: str, await_promise: bool = True, return_by_value: bool = True) -> EvaluateResponse: ...
 @overload
 async def execute_script(self, script: str, element: WebElement, await_promise: bool = True, return_by_value: bool = True) -> CallFunctionOnResponse: ...

Oh, I completely forgot about this PR due to the delayed response, and you probably closed it because of inactivity. If that’s the case, it would’ve been better to just ping me instead of closing it. I’ve just submitted the latest commit with the requested changes, so please consider reopening this PR. Also, let me explain the situation a little better.

The current code from the PR uses explicit methods. Tab.execute_script is clearly intended for page-level JavaScript execution, while WebElement.execute_script is meant for element-specific operations. Both rely on different runtime functions and return different data types, so combining them would blur their distinct use cases and create confusion. Since the WebElement class already implements this function, duplicating it in WebElement isn’t necessary.

1. Tab.execute_script (in pydoll/browser/tab.py):

async def execute_script(
   self,
   script: str,
   *,
   return_by_value: Optional[bool] = None,
   await_promise: Optional[bool] = None,
) -> EvaluateResponse:

2. WebElement.execute_script (in pydoll/elements/web_element.py):

async def execute_script(
    self,
    script: str,
    *,
    return_by_value: Optional[bool] = None,
    await_promise: Optional[bool] = None,
) -> CallFunctionOnResponse:

Honestly, it’d be great to keep the other arguments, as having the flexibility to tweak these parameters could be useful in some situations.

Hello @3xp0rt, thanks for the feedback. We have to keep the API consistent, otherwise it'll be a breaking change. I understand your point of view, and that makes sense. My idea would be: if the user pass an WebElement to the Tab.execute_script method, we can show a warning message informing that this is deprecated, and to use WebElement.execute_script instead;

For now, we can keep all the arguments, but we should preserve in just one method for clarity. Lets keep just execute_script method, and use overload for correct typing inference:

    
    @overload    # this is the signature without passing an WebElement as argument;
    async def execute_script(
        self,
        script: str,
        *,
        object_group: Optional[str] = None,
        include_command_line_api: Optional[bool] = None,
        silent: Optional[bool] = None,
        context_id: Optional[int] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        throw_on_side_effect: Optional[bool] = None,
        timeout: Optional[float] = None,
        disable_breaks: Optional[bool] = None,
        repl_mode: Optional[bool] = None,
        allow_unsafe_eval_blocked_by_csp: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> EvaluateResponse:
    
    @overload
    async def execute_element(    # this is the signature passing an WebElement as arg
        self,
        script: str,
        element: WebElement,
        *,
        arguments: Optional[list[CallArgument]] = None,
        silent: Optional[bool] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        execution_context_id: Optional[int] = None,
        object_group: Optional[str] = None,
        throw_on_side_effect: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> CallFunctionOnResponse:


    async def execute_element(    # this is the actual implementation
        self,
        script: str,
        element: Optional[WebElement] = None,
        *,
        arguments: Optional[list[CallArgument]] = None,
        silent: Optional[bool] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        execution_context_id: Optional[int] = None,
        object_group: Optional[str] = None,
        throw_on_side_effect: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> Union[EvaluateResponse, CallFunctionOnResponse]:

What do you think? Just make sure to document every param properly

Hello @3xp0rt, thanks for the feedback. We have to keep the API consistent, otherwise it'll be a breaking change. I understand your point of view, and that makes sense. My idea would be: if the user pass an WebElement to the Tab.execute_script method, we can show a warning message informing that this is deprecated, and to use WebElement.execute_script instead;

For now, we can keep all the arguments, but we should preserve in just one method for clarity. Lets keep just execute_script method, and use overload for correct typing inference:

    
    @overload    # this is the signature without passing an WebElement as argument;
    async def execute_script(
        self,
        script: str,
        *,
        object_group: Optional[str] = None,
        include_command_line_api: Optional[bool] = None,
        silent: Optional[bool] = None,
        context_id: Optional[int] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        throw_on_side_effect: Optional[bool] = None,
        timeout: Optional[float] = None,
        disable_breaks: Optional[bool] = None,
        repl_mode: Optional[bool] = None,
        allow_unsafe_eval_blocked_by_csp: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> EvaluateResponse:
    
    @overload
    async def execute_element(    # this is the signature passing an WebElement as arg
        self,
        script: str,
        element: WebElement,
        *,
        arguments: Optional[list[CallArgument]] = None,
        silent: Optional[bool] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        execution_context_id: Optional[int] = None,
        object_group: Optional[str] = None,
        throw_on_side_effect: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> CallFunctionOnResponse:


    async def execute_element(    # this is the actual implementation
        self,
        script: str,
        element: Optional[WebElement] = None,
        *,
        arguments: Optional[list[CallArgument]] = None,
        silent: Optional[bool] = None,
        return_by_value: Optional[bool] = None,
        generate_preview: Optional[bool] = None,
        user_gesture: Optional[bool] = None,
        await_promise: Optional[bool] = None,
        execution_context_id: Optional[int] = None,
        object_group: Optional[str] = None,
        throw_on_side_effect: Optional[bool] = None,
        unique_context_id: Optional[str] = None,
        serialization_options: Optional[SerializationOptions] = None,
    ) -> Union[EvaluateResponse, CallFunctionOnResponse]:

What do you think? Just make sure to document every param properly

Hello! Your idea is great, which is why I tried to implement it in the recent commits. I also added Chromium paths for Linux to fix a workflow issue I was experiencing. Please review the changes.